Let's create a base class (something like SampledOperation), which would hold the RANDOM_SEED and DEFAULT_SAMPLE_SIZE.

CountWhitespace feels the wrong name, probably should be CountNonTrivialWhitespace.

Lets create a record type here.

record DataQualityMetrics(Long untrimmedCount, Long whitespaceCount);

We can then do it as sinlge CompletableFuture to compute the values.

Set<Char> should work here I think.

The variable name makes me think it is good not bad.

We can then loop over the characters of the String and return true if the set contains any of them.

I was also confused by the variable name, shouldn't it be nonTrivialWhiteSpace instead?

Also, I imagine the JVM is good at optimizing such things, but perhaps we will make it easier for it if we make the list (or Set) a private static final field ensuring that it is computed only once upon initialization and not on every invocation of this function (since this function may be invoked millions of times).

I imagine the JIT would fold this constant at some point, but making this a constant from the beginning maybe would just be better.

Other thing - how do we know that this list is comprehensive?

Checking e.g. https://jkorpela.fi/chars/spaces.html suggests that we are missing e.g. the NARROW NO-BREAK SPACE U+202F or U+180E - the "MONGOLIAN VOWEL SEPARATOR" 😅

I think that as long as we can reasonably avoid it, I'd rather be against including such lists of constants in our code - it is easy for them to miss some obscure constants, and they are a bit less future proof. If instead we rely on ICU4J or the Java SDK itself, if there are new Unicode versions released in the future we'd just need to rely on them to get updated to the latest version instead of having to update many of such constant lists manually (which we will probably never do unless users start reporting bugs).

Of course things like "MONGOLIAN VOWEL SEPARATOR" are probably not that commonly used in most cases and missing them probably won't be a huge deal for a generic metric. But if we can use existing dependencies that we rely on anyway, I'd suggest preferring that.

Thus we could rewrite the code to essentially:

• iterate over every character in the text,
• check if it is any whitespace using UCharacter::isUWhiteSpace,
• if it is whitespace - then check if it is 'non-trivial' - a non trivial whitespace is any whitespace that is not the " " character.

for (...) {
    char c = str.charAt(i);
    if (UCharacter::isUWhiteSpace(c)) {
        boolean isNonTrivial = c != ' ';
        if (isNonTrivial) { return true; }
    }
    context.safepoint();
}

Ah sorry small correction - after further reading perhaps the two examples I provided above may actually not be classified as whitespace by the Unicode standard... So perhaps the List was good.

But I think we can still simplify the code by relying on isUWhiteSpace. It will also likely be much faster as it relies on bit-fiddling and boils down to 1 bit check per character instead of 16 independent text searches.

Let's change it so we check the count only once. Tiny nit.

Yeah it would be much better if we go the name_extra route and avoid the duplication in all the percentage_value entries.

I think Mixed implies is_text.not

No but this suggestion is not equivalent:

(self.value_type == Value_Type.Mixed || self.value_type.is_text).not
===
self.value_type != Value_Type.Mixed && self.value_type.is_text.not
!==
self.value_type.is_text.not

The counter-example is a mixed type column - self.value_type == Value_Type.Mixed.

Then the first and the equivalent second expression will evaluate to False but your suggested (third) expression evaluates to True.

I guess it becomes clearer if the condition is named: